Skip to content

[#494] Replace lettering prompt-copy UX with AI draft bubble generation#497

Merged
realproject7 merged 3 commits into
mainfrom
task/494-ai-draft-bubble-generation
Jun 7, 2026
Merged

[#494] Replace lettering prompt-copy UX with AI draft bubble generation#497
realproject7 merged 3 commits into
mainfrom
task/494-ai-draft-bubble-generation

Conversation

@realproject7

@realproject7 realproject7 commented Jun 7, 2026

Copy link
Copy Markdown
Owner

Closes #494.

Summary

  • move AI lettering assistance into the full cut review board with per-cut and batch AI draft actions for eligible cartoon cuts
  • generate first-pass editable overlays directly from the cut script, persist them through the same cuts JSON update path, and open the focused editor for tuning
  • track draft-ready vs user-edited cut status so writers can tell which cuts still need review before export/upload
  • remove prompt-copy as the primary focused-editor AI workflow and keep the editor centered on tuning overlays, manual placement, and export

Verification

  • npm run typecheck
  • npm run lint -- --quiet app/lib/lettering-status.ts app/lib/lettering-status.test.ts app/lib/cuts.ts app/web/components/CutListPanel.tsx app/web/components/CutListPanel.test.tsx app/web/components/LetteringEditor.tsx app/web/components/LetteringEditor.test.tsx
  • npm run app:build
  • targeted Vitest remains blocked in this environment before test collection by Unknown system error -122, write, including a single-fork run

Risks

  • the new AI-draft state is stored on each cut record (aiDraft metadata + generated overlays), so regressions would most likely surface in cut status labeling or editor save transitions
  • batch drafting intentionally skips cuts that already have overlays; the overwrite path is per-cut only and guarded by explicit confirmation
  • tracked app/web/dist artifacts were rebuilt for the UI change and are included in this PR

project7-interns
project7-interns requested changes Jun 7, 2026
View reviewed changes

@project7-interns project7-interns left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: REQUEST CHANGES

Summary

The #494 implementation is directionally aligned with the issue: AI draft actions now live in the cut review board, generated overlays are saved through the normal cuts JSON path, the focused editor opens for review/tuning, and per-cut overwrite is guarded by explicit confirmation. I did not find a product-code blocker in the reviewed source, but the live PR cannot be approved in its current GitHub state.

Findings

  • [high] The live PR is not mergeable against main; GitHub reports mergeStateStatus: DIRTY for head 05d67ab3ac65e542051eabdc730b72f92991637c.

    • File: N/A (branch state)
    • Suggestion: rebase/merge current main, resolve conflicts, push the updated head, and request re-review on the new commit.

  • [medium] No GitHub checks are reported for the PR head (gh pr checks 497 returns no checks for task/494-ai-draft-bubble-generation).

    • File: N/A (CI state)
    • Suggestion: ensure the expected check suite runs and is green on the updated head. Local typecheck/lint/build is useful context, but approval should be against a live, mergeable head with reported checks when available.

Decision

Requesting changes until the PR is mergeable and the live check state is resolved. After that, I can re-review the updated head; the current source review did not surface a separate functional blocker against issue #494.

project7-interns
project7-interns requested changes Jun 7, 2026
View reviewed changes

@project7-interns project7-interns left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: REQUEST CHANGES

Summary

The AI draft overlay flow itself is scoped correctly for issue #494: drafting starts from full cut review, generated overlays persist through the cuts JSON path, the focused editor is now positioned as the tuning surface, and overwrite confirmation exists for per-cut re-drafting. I am not approving the live PR head because the branch is currently stale/unmergeable against main and GitHub is reporting no live checks on the reviewed commit.

Findings

  • [medium] The live PR head 05d67ab3ac65e542051eabdc730b72f92991637c is behind main and GitHub marks the PR mergeable: false / mergeable_state: dirty, so this branch is not mergeable as reviewed.
    • Suggestion: rebase or merge main into the branch, resolve the resulting conflicts, and request re-review on the new head.
  • [medium] Live commit status for 05d67ab3ac65e542051eabdc730b72f92991637c reports state: pending with total_count: 0, and gh pr checks 497 reports no checks on the branch.
    • Suggestion: push the refreshed branch and make sure the expected CI checks are attached to the current PR head before requesting approval.

Decision

Requesting changes until the PR is updated to a mergeable head with live checks attached.

@realproject7 realproject7 force-pushed the task/494-ai-draft-bubble-generation branch from 05d67ab to 50e48eb Compare June 7, 2026 22:07
project7-interns
project7-interns approved these changes Jun 7, 2026
View reviewed changes

@project7-interns project7-interns left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: APPROVE

Summary

I re-reviewed PR #497 on the rebased live head 50e48eb202be45ac5f59b0b3eb31a969fa672d53. The earlier blockers were PR state only; on this head the branch is mergeable again, the expected lint-and-typecheck check is attached, and the AI-draft lettering flow remains aligned with issue #494.

Findings

  • No blocking findings.

Decision

Approved on GitHub. I spot-checked the rebased diff for the #494 flow and did not find a merge-resolution regression in the AI draft actions, aiDraft persistence, overwrite confirmation, or focused-editor tuning path. lint-and-typecheck was still pending during review, so merge should still use the live current head/check state.

project7-interns
project7-interns requested changes Jun 7, 2026
View reviewed changes

@project7-interns project7-interns left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: REQUEST CHANGES

Summary

The rebased PR head 50e48eb202be45ac5f59b0b3eb31a969fa672d53 resolves the prior dirty/mergeability blocker and the #493/#494 source paths I re-checked still look compatible. However, live CI is red on the current head.

Findings

  • [medium] Required lint-and-typecheck fails during npm test on the current PR head. The job reports 3 failing tests: CutListPanel cannot find cut-list-panel because the rendered state is cuts-error / Invalid cuts file, and two LetteringEditor tests now receive an extra second null argument from onSave.
    • File: app/web/components/CutListPanel.test.tsx:2333
    • File: app/web/components/LetteringEditor.test.tsx:589
    • File: app/web/components/LetteringEditor.test.tsx:643
    • Suggestion: update the affected tests/fixtures or implementation so the full npm test suite passes on GitHub, then request re-review on the new head.

Decision

Requesting changes until live lint-and-typecheck is green on the current PR head. I did not find a separate product-code blocker in the reviewed rebased source.

project7-interns
project7-interns approved these changes Jun 7, 2026
View reviewed changes

@project7-interns project7-interns left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: APPROVE

Summary

I re-reviewed PR #497 on follow-up head 0cbd41dcaff8daddd373706af58e815481d3f4d0. This update is narrowly scoped to the failing tests called out in the previous CI run: it fixes the rebased CutListPanel mock signature and updates the two LetteringEditor onSave expectations for the current callback shape.

Findings

  • No blocking findings.

Decision

Approved on GitHub for commit 0cbd41dcaff8daddd373706af58e815481d3f4d0. The product scope for #494 is unchanged; this head only adjusts tests to match the already-reviewed implementation. lint-and-typecheck was still pending during review, so merge should still use the live current head/check state.

project7-interns
project7-interns requested changes Jun 7, 2026
View reviewed changes

@project7-interns project7-interns left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: REQUEST CHANGES

Summary

The follow-up on 0cbd41dcaff8daddd373706af58e815481d3f4d0 is test-only, but it missed the failing CutListPanel mock that was called out in the prior review. The PR still cannot be approved as-is.

Findings

  • [medium] opts is referenced inside the AI-draft CutListPanel test mock, but that mock still declares only (url: string). This leaves the intended PUT branch broken and should also fail typecheck because opts is not in scope.
    • File: app/web/components/CutListPanel.test.tsx:2265
    • File: app/web/components/CutListPanel.test.tsx:2305
    • Suggestion: update this mock signature to accept opts?: RequestInit as well, matching the other fixed mocks, then rerun/confirm live lint-and-typecheck.

Decision

Requesting changes until this remaining test fix is applied and the live GitHub check is green on the current head.

project7-interns
project7-interns approved these changes Jun 7, 2026
View reviewed changes

@project7-interns project7-interns left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: APPROVE

Summary

I re-reviewed PR #497 on follow-up head eb1bfbfaea535bf589afb8a1c423f5d2da432670. This update is narrowly scoped to the remaining CutListPanel test mock-signature fix that was called out in the prior review.

Findings

  • No blocking findings.

Decision

Approved on GitHub for commit eb1bfbfaea535bf589afb8a1c423f5d2da432670. The product scope for #494 is unchanged; this head only corrects the final test mock signature. lint-and-typecheck was still pending during review, so merge should still use the live current head/check state.

project7-interns
project7-interns approved these changes Jun 7, 2026
View reviewed changes

@project7-interns project7-interns left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: APPROVE

Summary

Re-reviewed PR #497 on live head eb1bfbfaea535bf589afb8a1c423f5d2da432670. The latest follow-up is scoped to the remaining CutListPanel test mock signature, and it fixes the prior opts finding without changing the product implementation.

Findings

  • No blocking findings.

Decision

Approved on GitHub. The previously reviewed #494 AI draft lettering flow remains intact, the incremental diff only updates the affected test mock signature, and live lint-and-typecheck passes on the current head.

@realproject7 realproject7 merged commit 1871046 into main Jun 7, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@project7-interns project7-interns project7-interns approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Replace lettering prompt-copy UX with AI draft bubble generation

2 participants

@realproject7 @project7-interns